Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Django 4.x Support #176

Merged
merged 9 commits into from
Feb 4, 2022
Merged

Django 4.x Support #176

merged 9 commits into from
Feb 4, 2022

Conversation

jwedel
Copy link

@jwedel jwedel commented Feb 1, 2022

This PR fixes #173 and updates to Django 4.0.

Things that have been changed:

  • Requirements updated to latest
  • ugettext (replaced by gettext)
  • url (replaced by re_path)
  • USE_TZ (was set to False as the default values has been changes to True in D4.0. False keeps the <4.0 behaviour.)

I've added a new tox env from Django 4.0 with Python 3.8 + 3.9 as previous versions are not supported anymore.

tox reports:

ERROR:  py36-django22: InterpreterNotFound: python3.6
ERROR:  py37-django22: InterpreterNotFound: python3.7
  py38-django22: commands succeeded
ERROR:   py39-django22: could not install deps [pillow==6.2.2, -r/Users/janwedel/Development/repos/django-image-cropping/example/requirements.txt, Django>=2.2,<3.0]; v = InvocationError("/Users/janwedel/Development/repos/django-image-cropping/.tox/py39-django22/bin/python -m pip install pillow==6.2.2 -r/Users/janwedel/Development/repos/django-image-cropping/example/requirements.txt 'Django>=2.2,<3.0'", 1)
ERROR:  py36-django30: InterpreterNotFound: python3.6
ERROR:  py37-django30: InterpreterNotFound: python3.7
  py38-django30: commands succeeded
ERROR:   py39-django30: could not install deps [pillow==6.2.2, -r/Users/janwedel/Development/repos/django-image-cropping/example/requirements.txt, Django>=3.0,<3.1]; v = InvocationError("/Users/janwedel/Development/repos/django-image-cropping/.tox/py39-django30/bin/python -m pip install pillow==6.2.2 -r/Users/janwedel/Development/repos/django-image-cropping/example/requirements.txt 'Django>=3.0,<3.1'", 1)
ERROR:  py36-django31: InterpreterNotFound: python3.6
ERROR:  py37-django31: InterpreterNotFound: python3.7
  py38-django31: commands succeeded
ERROR:   py39-django31: could not install deps [pillow==6.2.2, -r/Users/janwedel/Development/repos/django-image-cropping/example/requirements.txt, Django>=3.1,<3.2]; v = InvocationError("/Users/janwedel/Development/repos/django-image-cropping/.tox/py39-django31/bin/python -m pip install pillow==6.2.2 -r/Users/janwedel/Development/repos/django-image-cropping/example/requirements.txt 'Django>=3.1,<3.2'", 1)
ERROR:  py36-django32: InterpreterNotFound: python3.6
ERROR:  py37-django32: InterpreterNotFound: python3.7
  py38-django32: commands succeeded
ERROR:   py39-django32: could not install deps [pillow==6.2.2, -r/Users/janwedel/Development/repos/django-image-cropping/example/requirements.txt, Django>=3.2,<4]; v = InvocationError("/Users/janwedel/Development/repos/django-image-cropping/.tox/py39-django32/bin/python -m pip install pillow==6.2.2 -r/Users/janwedel/Development/repos/django-image-cropping/example/requirements.txt 'Django>=3.2,<4'", 1)
  py38-django40: commands succeeded
ERROR:   py39-django40: could not install deps [pillow==6.2.2, -r/Users/janwedel/Development/repos/django-image-cropping/example/requirements.txt, Django>=4.0,<4.1]; v = InvocationError("/Users/janwedel/Development/repos/django-image-cropping/.tox/py39-django40/bin/python -m pip install pillow==6.2.2 -r/Users/janwedel/Development/repos/django-image-cropping/example/requirements.txt 'Django>=4.0,<4.1'", 1)

I don't have all python versions installed, but surprisingly, the tests actually work even on Django 2.2. Didn't expect that.

I also change the purest config as it was pointing to a non existing tests directory. Not sure if this was intentional or not.

Jan Wedel added 4 commits February 1, 2022 20:56
- Requirements updated to latest
- ugettext
- url
- USE_TZ
- Sorts imports in urls.py to fix isort issue
@anrie
Copy link
Member

anrie commented Feb 2, 2022

Hey @jwedel,
thanks for your efforts to make the project ready for 4.0.

It's okay that you don't have all Python versions locally installed, but we should make sure all the checks for all versions pass on CI which they currently don't: https://github.com/jonasundderwolf/django-image-cropping/runs/5033268821?check_suite_focus=true It would be great if you could fix this.

example/urls.py Show resolved Hide resolved
image_cropping/backends/base.py Show resolved Hide resolved
pytest.ini Show resolved Hide resolved
@jwedel
Copy link
Author

jwedel commented Feb 2, 2022

@anrie As I mentioned already in my PR description, I ran tox and it was successful on Django 2.2.

The problem with the build was that the dependency coverage==6.3 is only available for Python >= 3.7.

ERROR: Could not find a version that satisfies the requirement coverage==6.3

The build was running on 3.6 and then aborted before running with 3.7 and higher.

So, maybe this release with Django 4 support is a good time to cut support at least for Python 3.6?

Please, could you let tox run until the end so we can see if it works on Python >= 3.7?

EDIT: I will just remove 3.6 from the tox config to see if it would work. If you really want to get it running with Python 3.6, we/I need to find out which dependencies do not work with Python 3.6. and then downgrade.

- Also removes Pillow dep as it is not required
@jwedel
Copy link
Author

jwedel commented Feb 2, 2022

@anrie
So, in my latest commit, as mentioned above, I have removed:

  • Python 3.6 support from tox
  • Fixed coverage report
  • Remove pillow dependency (It was causing issues on my M1 Mac and tests still work after removing, so I think its not required)

@anrie
Copy link
Member

anrie commented Feb 3, 2022

Hey @jwedel,

the removal of the pillow dependency seems fine to me. easy-thumbnails seems to require a recent version of Pillow anyway these days.

I am a bit more hesitant when it comes to Python 3.6. I see the reasoning of the maintainers of coverage.py, that Python 3.6 has reached end-of-life. On the other hand Django 3.2 LTS is still around for a while and supports Python 3.6: https://docs.djangoproject.com/en/4.0/releases/3.2/#python-compatibility

And coverage is only used internally. Do you see any problem with staying on coverage.py 6.2? Otherwise i would prefer to keep the tests for 3.6 and only generate the coverage for recent versions, where coverage is supported.

@jwedel
Copy link
Author

jwedel commented Feb 3, 2022

@anrie no, I just don’t know if other deps also need to be downgraded again. And I’m actually having trouble to install python 3.6 on my Mac, installation/build fails so I cannot really test it locally. So I have to rely on the CI here. Can you somehow enable that the builds run on a commit? This would make the feedback loop a bit shorter for me.

I am wondering why the build still builds with python 3.6 even though I have removed it from tox config.

But I will add it again and downgrade coverage.

Let’s see…

@anrie
Copy link
Member

anrie commented Feb 3, 2022

@jwedel I think the builds still run because of this.

And as the 3.6-Build fails, the other jobs are aborted too.
So for testing on your branch you should be able to adjust the build strategy.

@jwedel
Copy link
Author

jwedel commented Feb 3, 2022

@anrie

  • I downgraded coverage to 5.5 and checked all other updated libs if they theoretically support 3.6 and they do.
  • I added 3.6 support to tox.ini
  • I tried to add 3.10 support (why not?) but I was not allows to push changes to the GitHub actions file (understandably) and had to revert the changes. If you want that, you need to do it yourself.
  • I also saw this commented-out section in the pipeline file which could be reenabled as I have fixed the coverage report:

# TODO: re-enable when coverage is fixed (see tox.ini)
# - name: Report to coverall # by the package as the action is broken, but won't work on external forks!
# if: matrix.python-version == 3.7
# env:
# COVERALLS_REPO_TOKEN: ${{ secrets.COVERALLS_REPO_TOKEN }}
# run: |
# pip install coveralls
# coveralls
# The official action is broken as only supports LCOV format, not supported by python coverage
# Think about moving to codecov or another provider?
# - name: Coveralls
# uses: coverallsapp/github-action@master
# with:
# github-token: ${{ secrets.GITHUB_TOKEN }}

@anrie
Copy link
Member

anrie commented Feb 3, 2022

Hey @jwedel,

I guess we are close. Alle checks pass, it's just that the tests for 3.6 don't run at the moment: https://github.com/jonasundderwolf/django-image-cropping/runs/5057844917?check_suite_focus=true

Looks like your final commit/merge removed 3.6 from the envlist in tox again: https://github.com/jonasundderwolf/django-image-cropping/pull/176/files

@jwedel
Copy link
Author

jwedel commented Feb 3, 2022

@anrie Damnit, I used some git wizardry to revers parts of my commit and messed it up... Hopefully, it's better now ^^

@anrie
Copy link
Member

anrie commented Feb 4, 2022

@jwedel Looks better now 👍

Thanks a lot for your contribution!
I merge this PR now and issue a new release after I had the chance to look a the remaining issues (coverage, Python 3.10).

@anrie anrie merged commit 552a8d9 into jonasundderwolf:master Feb 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Django 4 support
2 participants